<html>
<head><meta charset="utf-8"><title>proposed perf regression policy · t-compiler · Zulip Chat Archive</title></head>
<h2>Stream: <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/index.html">t-compiler</a></h2>
<h3>Topic: <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/proposed.20perf.20regression.20policy.html">proposed perf regression policy</a></h3>

<hr>

<base href="https://rust-lang.zulipchat.com">

<head><link href="https://rust-lang.github.io/zulip_archive/style.css" rel="stylesheet"></head>

<a name="240442028"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/proposed%20perf%20regression%20policy/near/240442028" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> nikomatsakis <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/proposed.20perf.20regression.20policy.html#240442028">(May 27 2021 at 09:50)</a>:</h4>
<p>Hey <span class="user-mention" data-user-id="224872">@rylev</span>, <span class="user-mention" data-user-id="116122">@simulacrum</span>, <span class="user-mention" data-user-id="116083">@pnkfelix</span>, <span class="user-mention" data-user-id="125250">@Wesley Wiser</span> and anyone else who is interested:</p>
<p>I was thinking about our policy around compilation time regressions or lack thereof. I think it'd be useful if we had some criteria for when we deem regressions acceptable and a simple policy to make people aware when we are landing a known regression and why. Here is my first attempt at a quick write-up:</p>
<p><a href="https://hackmd.io/pMbSMx-PTnCmKlhY3X-Wqg">https://hackmd.io/pMbSMx-PTnCmKlhY3X-Wqg</a></p>
<p>I don't have the time to carry this over the finish line, but I thought maybe one of you might like to take a look and turn it into an MCP and (eventually) an edit to forge.</p>
<p>I know I would appreciate having a simple policy like this! It's always rather hard to decide when it's ok to r+ such a case.</p>



<a name="240446439"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/proposed%20perf%20regression%20policy/near/240446439" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> simulacrum <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/proposed.20perf.20regression.20policy.html#240446439">(May 27 2021 at 10:35)</a>:</h4>
<p>Generally seems like a good start. I think building in explicit check point of <em>some</em> positive feedback is good, and one strategy might be to have the PR i-nominated - anything that regresses would then be discussed. We shouldn't be landing regressions often enough to make that a problem, I think.</p>



<a name="240446572"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/proposed%20perf%20regression%20policy/near/240446572" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> simulacrum <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/proposed.20perf.20regression.20policy.html#240446572">(May 27 2021 at 10:37)</a>:</h4>
<p>Interesting "4th" reason worth talking about: "improving rustc compilation time" - this one likely wanting a cost/benefit with the runtime regressions.</p>



<a name="240450509"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/proposed%20perf%20regression%20policy/near/240450509" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> rylev <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/proposed.20perf.20regression.20policy.html#240450509">(May 27 2021 at 11:17)</a>:</h4>
<p>Looks good - I'm a little worried about the 10 day wait period for the general reason of not wanting to discourage people from running the perf test in the first place. I don't want us to get into a situation where people (even subconsciously) avoid running the perf suite because they don't want to risk having their changes delayed.</p>



<a name="240450600"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/proposed%20perf%20regression%20policy/near/240450600" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> rylev <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/proposed.20perf.20regression.20policy.html#240450600">(May 27 2021 at 11:18)</a>:</h4>
<p>I think we can easily make the proposed process here work well with any changes that we make to the process automation (which we discussed a few weeks back but which has been implemented yet due to lack of time)</p>



<a name="240451306"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/proposed%20perf%20regression%20policy/near/240451306" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> nagisa <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/proposed.20perf.20regression.20policy.html#240451306">(May 27 2021 at 11:26)</a>:</h4>
<p>The risk is already there though, isn't it?</p>



<a name="240464601"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/proposed%20perf%20regression%20policy/near/240464601" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> nikomatsakis <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/proposed.20perf.20regression.20policy.html#240464601">(May 27 2021 at 13:12)</a>:</h4>
<p><span class="user-mention silent" data-user-id="224872">rylev</span> <a href="#narrow/stream/131828-t-compiler/topic/proposed.20perf.20regression.20policy/near/240450509">said</a>:</p>
<blockquote>
<p>Looks good - I'm a little worried about the 10 day wait period for the general reason of not wanting to discourage people from running the perf test in the first place. I don't want us to get into a situation where people (even subconsciously) avoid running the perf suite because they don't want to risk having their changes delayed.</p>
</blockquote>
<p>I tend to thnk the 10 day was too much</p>



<a name="240464636"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/proposed%20perf%20regression%20policy/near/240464636" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> nikomatsakis <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/proposed.20perf.20regression.20policy.html#240464636">(May 27 2021 at 13:13)</a>:</h4>
<p>but maybe if we have some way to "affirmatively" decide faster, that's good</p>



<a name="240464694"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/proposed%20perf%20regression%20policy/near/240464694" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> nikomatsakis <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/proposed.20perf.20regression.20policy.html#240464694">(May 27 2021 at 13:13)</a>:</h4>
<p><span class="user-mention silent" data-user-id="116122">simulacrum</span> <a href="#narrow/stream/131828-t-compiler/topic/proposed.20perf.20regression.20policy/near/240446572">said</a>:</p>
<blockquote>
<p>Interesting "4th" reason worth talking about: "improving rustc compilation time" - this one likely wanting a cost/benefit with the runtime regressions.</p>
</blockquote>
<p>i.e., sometimes improvng rustc time may harm other benchmarks?</p>



<a name="240464699"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/proposed%20perf%20regression%20policy/near/240464699" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> nikomatsakis <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/proposed.20perf.20regression.20policy.html#240464699">(May 27 2021 at 13:13)</a>:</h4>
<p>we can probably generalize that to:</p>



<a name="240464708"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/proposed%20perf%20regression%20policy/near/240464708" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> nikomatsakis <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/proposed.20perf.20regression.20policy.html#240464708">(May 27 2021 at 13:13)</a>:</h4>
<ul>
<li>uneven impact</li>
</ul>



<a name="240466716"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/proposed%20perf%20regression%20policy/near/240466716" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> simulacrum <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/proposed.20perf.20regression.20policy.html#240466716">(May 27 2021 at 13:27)</a>:</h4>
<p>Yeah, I think the interesting aspect of uneven impact is we don't have a good weighting strategy</p>



<a name="240468865"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/proposed%20perf%20regression%20policy/near/240468865" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> pnkfelix <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/proposed.20perf.20regression.20policy.html#240468865">(May 27 2021 at 13:42)</a>:</h4>
<p>Wait, the first sentence of this doc is "We try to avoid regressing compilation time.” This whole doc is solely about performance regressions <em>to</em> compilation time</p>



<a name="240468881"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/proposed%20perf%20regression%20policy/near/240468881" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> pnkfelix <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/proposed.20perf.20regression.20policy.html#240468881">(May 27 2021 at 13:42)</a>:</h4>
<p>right?</p>



<a name="240468959"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/proposed%20perf%20regression%20policy/near/240468959" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> pnkfelix <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/proposed.20perf.20regression.20policy.html#240468959">(May 27 2021 at 13:42)</a>:</h4>
<p>As in, we don’t even properly keep running metrics on the performance of the code we generate</p>



<a name="240469063"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/proposed%20perf%20regression%20policy/near/240469063" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> pnkfelix <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/proposed.20perf.20regression.20policy.html#240469063">(May 27 2021 at 13:43)</a>:</h4>
<p>(so its a little weird to add it as a trade-off that we are in a good position to weigh. I wish we <em>were</em> better able to weigh such trade-offs)</p>



<a name="240470762"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/proposed%20perf%20regression%20policy/near/240470762" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> simulacrum <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/proposed.20perf.20regression.20policy.html#240470762">(May 27 2021 at 13:55)</a>:</h4>
<p>I was referring to compile time (of third party code) as different from rustc itself</p>



<a name="240471643"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/proposed%20perf%20regression%20policy/near/240471643" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Wesley Wiser <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/proposed.20perf.20regression.20policy.html#240471643">(May 27 2021 at 14:00)</a>:</h4>
<p>I think the "uneven impact" question is one of the hardest to answer for me personally. If a PR is all green, it's very easy to r+ or if it's all red, it's very easy to say "we can't land this" and often fairly easy to say "we can land because of <code>x</code> reason" but when a PR is mixed it's very hard for me to tell if we should be approving it. This is especially true in cases where the purpose of the PR is to improve performance. I often personally weigh the "real world" crate benchmarks higher than the synthetic ones but even then, you can still have uneven results.</p>



<a name="240496198"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/proposed%20perf%20regression%20policy/near/240496198" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> scottmcm <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/proposed.20perf.20regression.20policy.html#240496198">(May 27 2021 at 16:39)</a>:</h4>
<p><span class="user-mention silent" data-user-id="116083">pnkfelix</span> <a href="#narrow/stream/131828-t-compiler/topic/proposed.20perf.20regression.20policy/near/240468959">said</a>:</p>
<blockquote>
<p>As in, we don’t even properly keep running metrics on the performance of the code we generate</p>
</blockquote>
<p>I've been occasionally concerned that the fact we can only measure compile time and not runtime performance means that we could be substantially regressing the other side for a 1% compile-time speedup.  This is less true for check builds, but it's not hard for a change to a library something to expose more optimization opportunities but get reported as a regression because all of a sudden LLVM is doing more work, making compilation slower.</p>



<a name="240497697"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/proposed%20perf%20regression%20policy/near/240497697" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Josh Triplett <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/proposed.20perf.20regression.20policy.html#240497697">(May 27 2021 at 16:50)</a>:</h4>
<p>I think that's a good example of where we should absolutely allow "regressions": additional optimizations.</p>



<a name="240497804"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/proposed%20perf%20regression%20policy/near/240497804" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Josh Triplett <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/proposed.20perf.20regression.20policy.html#240497804">(May 27 2021 at 16:51)</a>:</h4>
<p>In particular, if a regression only applies to release builds, and not to build with optimization disabled, and it has a runtime improvement that seems worth the cost, that seems entirely fine.</p>



<a name="240497809"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/proposed%20perf%20regression%20policy/near/240497809" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> scottmcm <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/proposed.20perf.20regression.20policy.html#240497809">(May 27 2021 at 16:51)</a>:</h4>
<p><span class="user-mention silent" data-user-id="116009">nikomatsakis</span> <a href="#narrow/stream/131828-t-compiler/topic/proposed.20perf.20regression.20policy/near/240442028">said</a>:</p>
<blockquote>
<p>I was thinking about our policy around compilation time regressions or lack thereof. I think it'd be useful if we had some criteria for when we deem regressions acceptable and a simple policy to make people aware when we are landing a known regression and why.</p>
</blockquote>
<p>My recent <code>?</code> PR might make an interesting case study.  It was green on some, but red on more, and I'm not sure it was a perfect match for any of the listed categories.  (I wouldn't call it a soundness fix, an improvement to the compiler architecture, or a simplification.)  But also the change actually generated <em>less</em> MIR, so there wasn't an obvious place to improve anything -- it would have needed a perf improvement to trait solving or something, but that also doesn't feel like it should be the same PR.</p>
<p>I do think it could have made sense to wait if it was, say, a 25% regression, but I don't know what to do about a maybe 0.8% regression like that.  Because it's certainly also true that we don't want to just allow a whole bunch of individually-tiny regressions to add up...</p>



<a name="240772352"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/proposed%20perf%20regression%20policy/near/240772352" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> scottmcm <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/proposed.20perf.20regression.20policy.html#240772352">(May 30 2021 at 20:25)</a>:</h4>
<p>Since I'm hitting this again, as an occasional compiler-PR-maker it'd be nice to have a more obvious "this counts as a regression" marker for when one would need to follow the policy. My <a href="https://perf.rust-lang.org/compare.html?start=bff138dbd95cec763f4def6b91bb465a26aaad9f&amp;end=4fc737828e7a2f978c30875c76c93b2f0e5d6562">current PR results</a> have some red, some green.  None of the numbers are huge.  Is it a regression?  I don't know.</p>



<a name="240784004"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/proposed%20perf%20regression%20policy/near/240784004" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> simulacrum <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/proposed.20perf.20regression.20policy.html#240784004">(May 31 2021 at 01:54)</a>:</h4>
<p>I don't think that's quite the right question - it does seem pretty clearly a regression, noise on those benchmarks is usually much less than 0.1% so these are likely significant changes. Whether it's acceptable is a separate question.</p>



<a name="240787797"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/proposed%20perf%20regression%20policy/near/240787797" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> scottmcm <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/proposed.20perf.20regression.20policy.html#240787797">(May 31 2021 at 03:37)</a>:</h4>
<p>I guess then the other side is asking for is more guidance on how to look into small regressions, then.  For example, <code>many-assoc-items</code> shows 0.0% in the noise run, and 0.3% worse in my run, so ok, that sounds like maybe it's real.  And certainly if I'd been touching name resolution, or some visitor over items, or something then I'd have a place to start.  But the PR I'm doing only affects (AFAIK) equality for arrays of primitive types, and <a href="https://github.com/rust-lang/rustc-perf/blob/master/collector/benchmarks/many-assoc-items/src/lib.rs">that test</a> never uses arrays nor <code>==</code>.  So coupled with other builds being reported as slightly faster, it makes it subjectively <em>feel</em> like noise.</p>



<a name="240823847"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/proposed%20perf%20regression%20policy/near/240823847" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> simulacrum <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/proposed.20perf.20regression.20policy.html#240823847">(May 31 2021 at 11:53)</a>:</h4>
<p>We're testing compiler performance, though. Usually you can identify causes of regressions or improvements on a particular benchmark with cachegrind.</p>



<hr><p>Last updated: Aug 07 2021 at 22:04 UTC</p>
</html>